Skip to content

Don't delete Canvas tokens as often#359

Open
dspencer001 wants to merge 4 commits intomainfrom
ds_dont_delete_tokens
Open

Don't delete Canvas tokens as often#359
dspencer001 wants to merge 4 commits intomainfrom
ds_dont_delete_tokens

Conversation

@dspencer001
Copy link
Copy Markdown
Contributor

No description provided.

Deleting Canvas tokens causes some of our downstream apps a lot of heartache. We almost never want to delete them.
We don't want to delete tokens when Canvas might succeed later at refreshing.
:recoverable, :rememberable, :trackable, :validatable, :omniauthable

has_many :authentications, dependent: :destroy, inverse_of: :user
has_many :authentications, inverse_of: :user
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rails/HasManyOrHasOneDependent: Specify a :dependent option.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we do want this change, we should probably use dependent: :nullify https://guides.rubyonrails.org/association_basics.html#dependent

@@ -1,3 +1,3 @@
class CanvasCourse < ApplicationRecord
has_many :authentications, dependent: :destroy, inverse_of: :canvas_course
has_many :authentications, inverse_of: :canvas_course
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rails/HasManyOrHasOneDependent: Specify a :dependent option.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we do want this change, we should probably use dependent: :nullify https://guides.rubyonrails.org/association_basics.html#dependent

@bfcoder
Copy link
Copy Markdown
Collaborator

bfcoder commented May 15, 2020

@dspencer001 What scenario do you have in mind that you would want the authentication to stick around after either the course or user model are deleted?

@dspencer001
Copy link
Copy Markdown
Contributor Author

@bfcoder We have users getting unexpectedly deleted in Search, and probably other places as well. In the cases I've noticed it is the user we create specifically for search, so there shouldn't be any reason for that user to be deleted. This would let the token at least stick around and not break anything that depends on it.

@bfcoder
Copy link
Copy Markdown
Collaborator

bfcoder commented May 15, 2020

Interesting. ok, well I've left a comment on the dependent: :destroy stuff as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants